replace generated Archtest by TechnicalStructureTest
#17521
Conversation
|
|
codecholeric
force-pushed
the
extend-arch-test
branch
from
7d4121a
to
9a4f315
Compare
|
When I forgot to push |
codecholeric
force-pushed
the
extend-arch-test
branch
from
9a4f315
to
87b7f71
Compare
Since the generated application is modelled after a simple layered architecture we can provide a generated ArchUnit test that uses `Architectures.layeredArchitecture()` as a nice example of a higher abstraction architecture rule. This will assert even more dependencies for correctness than before (for example no dependencies from repositories to services). Furthermore, by specifying an exact list of which layer is allowed to access which other layer, instead of forbidding specific dependencies from one layer to another, we remove one potential source of accidental violation. E.g., by forbidding `service -> web` we can still have accesses like `services -> some_utils_no_one_cares_about -> web`. If we specify that no other layer may access web (because all accesses are via REST, etc., and not from other Java code), we can make sure that there are no accidental dependencies through some unexpected packages. I've also replaced the manual import via `new ClassFileImporter()...importPackages(..)` by ArchUnit's JUnit 5 support. The dependency was already correct anyway, and it allows to write the test a little more concisely and automatically brings caching between multiple rules that import the same classes. Resolves: jhipster#17520
codecholeric
force-pushed
the
extend-arch-test
branch
from
87b7f71
to
6b5f1ff
Compare
|
Hmm, I'm a little stuck I think. Because, the reactive configurations create the following dependency |
|
From my point of view, your arch tests are correct. If we have |
It’s open source, and community based. Unless we have good tests in place and enforce code standards it’s hard to keep good code standards every time. I think your PR looks correct, we should fix the implementation. |
|
@mshima Sure, feel free to do what you want! Does this mean you would take over the change from this PR and adjust the implementation then? In any case, just let me know if you want me to change something / take further action in any direction! |
|
Looks like there is only 1 failure. Should be easy to fix. |
|
#17542 wasn’t enough, moving to a branch at main repository. |
Since the generated application is modelled after a simple layered architecture we can provide a generated ArchUnit test that uses
Architectures.layeredArchitecture()as a nice example of a higher abstraction architecture rule. This will assert even more dependencies for correctness than before (for example no dependencies from repositories to services). Furthermore, by specifying an exact list of which layer is allowed to access which other layer, instead of forbidding specific dependencies from one layer to another, we remove one potential source of accidental violation. E.g., by forbiddingservice -> webwe can still have accesses likeservices -> some_utils_no_one_cares_about -> web. If we specify that no other layer may access web (because all accesses are via REST, etc., and not from other Java code), we can make sure that there are no accidental dependencies through some unexpected packages.I've also replaced the manual import via
new ClassFileImporter()...importPackages(..)by ArchUnit's JUnit 5 support. The dependency was already correct anyway, and it allows to write the test a little more concisely and automatically brings caching between multiple rules that import the same classes.Related: #17520
Please make sure the below checklist is followed for Pull Requests.
When you are still working on the PR, consider converting it to Draft (below reviewers) and adding
skip-cilabel, you can still see CI build result at your branch.